Handle special properties like __proto__#59
Handle special properties like __proto__#59rvagg wants to merge 1 commit intodominictarr:masterfrom rvagg:special-words
Conversation
|
Hmm, okay this is an important issue. What if instead sublevel just throws on reserved words (like functions from Object.prototype) |
|
This is how we deal with this exact problem in MemDOWN fwiw. I don't really mind how you deal with it here as long as it's dealt with. But, my opinion on throwing on use of a reserved word is that it would be a terribly leaky abstraction and generally a surprise to encounter it when you did. Would you document it in the README? Would you feel comfortable writing that documentation, admitting that it's a leaky abstraction? What's the damage of the break here for multilevel? It's only the properties on that |
|
Hmm, so wherether or not this is a leaky abstraction depends on what type of string is a valid sublevel name... I'll admit this was never clearly defined, but there are some implicit definitions... Consider event emitters, if you did this: This would have always caused an error with sublevel, so I guess this is the first time someone has tried to do this, but what the valid keys are should be documented... Lines 61 to 63 in 4ec1f68 And it would have returned the wrong object and given you a runtime type error. I guess this question boils down to, in javascript, is "common sense" to not use these values, or is it to support them? In memdown, that is a slightly different case, because a database should certainly be able to contain any value but a sublevel or an event is like a directory, it's better to choose a sensible value, and not use spaces, etc, etc. |
|
use property setter/getter can keep compatibility. see my branch: https://github.com/snowyu/level-sublevel/tree/hotfix/sublevels-compatibility. also I have opened a pull-request. |
Because you're using
sublevelsas a plain map of course, I happened across this because my sublevels were generated by external data and one was called'constructor', bzzt!This is a breaking change if you choose to accept it because
sublevelsis intended (I guess) to be exposed publicly.